fix(eth_sendRawTransaction): return InvalidParams for malformed RLP - #12003

Claude finished @manusw7's task in 2m 45s —— View job

PR Review: fix(eth_sendRawTransaction): return InvalidParams for malformed RLP

• Gathering context
• Reading relevant rules
• Reviewing changed files
• Analyzing correctness, security, and edge cases
• Posting findings

Summary

This is a correct, well-scoped fix. The change aligns Nethermind with JSON-RPC 2.0 semantics: parameter-decoding failures belong at -32602 (InvalidParams), not -32000 (TransactionRejected), which is the catch-all for runtime or application-level failures. The PR is consistent with Reth and Besu behavior.

Two low-severity nits are noted below; neither blocks merge.

Findings

Low — String concatenation instead of interpolation

EthRpcModule.cs:371 uses "Invalid RLP: " + e.Message. The rest of the file and the codebase consistently use $"…" interpolated strings for message construction. This is a pure style nit with no runtime impact (error paths are cold), but it's worth keeping consistent.

Fix this →

            return ResultWrapper<Hash256>.Fail($"Invalid RLP: {e.Message}", ErrorCodes.InvalidParams);

Low — SetCode null-address test asserts code only, not message

eth_sendRawTransaction_returns_correct_error_if_AuthorityTuple_has_null_value (line ~2416) only asserts actual.Error!.Code == InvalidParams but doesn't assert the message fragment. The parameterized test above it does check for "Invalid RLP:", so the pattern is established. Since this test was pre-existing (only the expected code changed), it's acceptable as-is, but adding a Does.Contain("Invalid RLP:") assertion would make it consistent with the new parameterized test.

Positive observations

• eth_sendRawTransactionSync is correctly covered — it delegates to the virtual eth_sendRawTransaction (line 466), so the fix propagates automatically; the SendRawTransactionSyncFailureCases update confirms this is also tested.
• e.Message content is safe to expose — all RlpException throw sites in the serializer use structural descriptions (positions, byte values, type names). None include raw user-supplied bytes or stack traces. No injection or information-disclosure risk.
• Good parameterization — the five test cases (c0, d4, 09c0, 03c0, 04c0) cover all the failure shapes from issue eth_sendRawTransaction: RLP decode failure returns wrong JSON-RPC error code #11921. Using Does.Contain for message assertions is appropriate here since the suffix differs per case.
• DebugRpcModule / TraceRpcModule are intentionally out of scope — those catch RlpException with InvalidInput for block-level RPC calls. That's a separate concern and correct not to change here.

@manusw7 ready for review?

@manusw7 ready for review?

we should wait for an update from geth before proceeding (see here ethereum/go-ethereum#35129)